Merge pull request #1444 from cantino/data_output_agent_limits_events_after_ordering

DataOutputAgent limits events after ordering

Akinori MUSHA 8 jaren geleden
bovenliggende
commit
9b23c28049

+ 14 - 12
app/concerns/sortable_events.rb

@@ -5,6 +5,8 @@ module SortableEvents
5 5
     validate :validate_events_order
6 6
   end
7 7
 
8
+  EVENTS_ORDER_KEY = 'events_order'.freeze
9
+
8 10
   def description_events_order(*args)
9 11
     self.class.description_events_order(*args)
10 12
   end
@@ -23,9 +25,9 @@ module SortableEvents
23 25
       !can_order_created_events?
24 26
     end
25 27
 
26
-    def description_events_order(events = 'events created in each run')
28
+    def description_events_order(events = 'events created in each run', events_order_key = EVENTS_ORDER_KEY)
27 29
       <<-MD.lstrip
28
-        To specify the order of #{events}, set `events_order` to an array of sort keys, each of which looks like either `expression` or `[expression, type, descending]`, as described as follows:
30
+        To specify the order of #{events}, set `#{events_order_key}` to an array of sort keys, each of which looks like either `expression` or `[expression, type, descending]`, as described as follows:
29 31
 
30 32
         * _expression_ is a Liquid template to generate a string to be used as sort key.
31 33
 
@@ -48,8 +50,8 @@ module SortableEvents
48 50
     self.class.cannot_order_created_events?
49 51
   end
50 52
 
51
-  def events_order
52
-    options['events_order']
53
+  def events_order(key = EVENTS_ORDER_KEY)
54
+    options[key]
53 55
   end
54 56
 
55 57
   module AutomaticSorter
@@ -102,8 +104,8 @@ module SortableEvents
102 104
   }
103 105
   EXPRESSION_TYPES = EXPRESSION_PARSER.keys.freeze
104 106
 
105
-  def validate_events_order
106
-    case order_by = events_order
107
+  def validate_events_order(events_order_key = EVENTS_ORDER_KEY)
108
+    case order_by = events_order(events_order_key)
107 109
     when nil
108 110
     when Array
109 111
       # Each tuple may be either [expression, type, desc] or just
@@ -113,29 +115,29 @@ module SortableEvents
113 115
         when String
114 116
           # ok
115 117
         else
116
-          errors.add(:base, "first element of each events_order tuple must be a Liquid template")
118
+          errors.add(:base, "first element of each #{events_order_key} tuple must be a Liquid template")
117 119
           break
118 120
         end
119 121
         case type
120 122
         when nil, *EXPRESSION_TYPES
121 123
           # ok
122 124
         else
123
-          errors.add(:base, "second element of each events_order tuple must be #{EXPRESSION_TYPES.to_sentence(last_word_connector: ' or ')}")
125
+          errors.add(:base, "second element of each #{events_order_key} tuple must be #{EXPRESSION_TYPES.to_sentence(last_word_connector: ' or ')}")
124 126
           break
125 127
         end
126 128
         if !desc.nil? && boolify(desc).nil?
127
-          errors.add(:base, "third element of each events_order tuple must be a boolean value")
129
+          errors.add(:base, "third element of each #{events_order_key} tuple must be a boolean value")
128 130
           break
129 131
         end
130 132
       end
131 133
     else
132
-      errors.add(:base, "events_order must be an array of arrays")
134
+      errors.add(:base, "#{events_order_key} must be an array of arrays")
133 135
     end
134 136
   end
135 137
 
136 138
   # Sort given events in order specified by the "events_order" option
137
-  def sort_events(events)
138
-    order_by = events_order.presence or
139
+  def sort_events(events, events_order_key = EVENTS_ORDER_KEY)
140
+    order_by = events_order(events_order_key).presence or
139 141
       return events
140 142
 
141 143
     orders = order_by.map { |_, _, desc = false| boolify(desc) }

+ 64 - 3
app/models/agents/data_output_agent.rb

@@ -46,9 +46,14 @@ module Agents
46 46
               "_contents": "tag contents (can be an object for nesting)"
47 47
             }
48 48
 
49
-        # Ordering events in the output
49
+        # Ordering events
50 50
 
51
-        #{description_events_order('events in the output')}
51
+        #{description_events_order('events')}
52
+
53
+        DataOutputAgent will select the last `events_to_show` entries of its received events sorted in the order specified by `events_order`, which is defaulted to the event creation time.
54
+        So, if you have multiple source agents that may create many events in a run, you may want to either increase `events_to_show` to have a larger "window", or specify the `events_order` option to an appropriate value (like `date_published`) so events from various sources are properly mixed in the resulted feed.
55
+
56
+        There is also an option `events_list_order` that only controls the order of events listed in the final output, without attempting to maintain a total order of received events.  It has the same format as `events_order` and is defaulted to `#{Utils.jsonify(DEFAULT_EVENTS_ORDER['events_list_order'])}` so the selected events are listed in reverse order like most popular RSS feeds list their articles.
52 57
 
53 58
         # Liquid Templating
54 59
 
@@ -176,6 +181,59 @@ module Agents
176 181
       interpolated['push_hubs'].presence || []
177 182
     end
178 183
 
184
+    DEFAULT_EVENTS_ORDER = {
185
+      'events_order' => nil,
186
+      'events_list_order' => [["{{_index_}}", "number", true]],
187
+    }
188
+
189
+    def events_order(key = SortableEvents::EVENTS_ORDER_KEY)
190
+      super || DEFAULT_EVENTS_ORDER[key]
191
+    end
192
+
193
+    def latest_events(reload = false)
194
+      events =
195
+        if (event_ids = memory[:event_ids]) &&
196
+           memory[:events_order] == events_order &&
197
+           memory[:events_to_show] >= events_to_show
198
+          received_events.where(id: event_ids).to_a
199
+        else
200
+          memory[:last_event_id] = nil
201
+          reload = true
202
+          []
203
+        end
204
+
205
+      if reload
206
+        memory[:events_order] = events_order
207
+        memory[:events_to_show] = events_to_show
208
+
209
+        new_events =
210
+          if last_event_id = memory[:last_event_id]
211
+            received_events.where(Event.arel_table[:id].gt(last_event_id)).
212
+              order(id: :asc).to_a
213
+          else
214
+            source_ids.flat_map { |source_id|
215
+              # dig twice as many events as the number of
216
+              # `events_to_show`
217
+              received_events.where(agent_id: source_id).
218
+                last(2 * events_to_show)
219
+            }.sort_by(&:id)
220
+          end
221
+
222
+        unless new_events.empty?
223
+          memory[:last_event_id] = new_events.last.id
224
+          events.concat(new_events)
225
+        end
226
+      end
227
+
228
+      events = sort_events(events).last(events_to_show)
229
+
230
+      if reload
231
+        memory[:event_ids] = events.map(&:id)
232
+      end
233
+
234
+      events
235
+    end
236
+
179 237
     def receive_web_request(params, method, format)
180 238
       unless interpolated['secrets'].include?(params['secret'])
181 239
         if format =~ /json/
@@ -185,7 +243,7 @@ module Agents
185 243
         end
186 244
       end
187 245
 
188
-      source_events = sort_events(received_events.order(id: :desc).limit(events_to_show).to_a)
246
+      source_events = sort_events(latest_events(), 'events_list_order')
189 247
 
190 248
       interpolation_context.stack do
191 249
         interpolation_context['events'] = source_events
@@ -252,6 +310,9 @@ module Agents
252 310
     def receive(incoming_events)
253 311
       url = feed_url(secret: interpolated['secrets'].first, format: :xml)
254 312
 
313
+      # Reload new events and update cache
314
+      latest_events(true)
315
+
255 316
       push_hubs.each do |hub|
256 317
         push_to_hub(hub, url)
257 318
       end

+ 6 - 2
app/models/agents/rss_agent.rb

@@ -82,8 +82,12 @@ module Agents
82 82
       validate_events_order
83 83
     end
84 84
 
85
-    def events_order
86
-      super.presence || DEFAULT_EVENTS_ORDER
85
+    def events_order(key = SortableEvents::EVENTS_ORDER_KEY)
86
+      if key == SortableEvents::EVENTS_ORDER_KEY
87
+        super.presence || DEFAULT_EVENTS_ORDER
88
+      else
89
+        raise ArgumentError, "unsupported key: #{key}"
90
+      end
87 91
     end
88 92
 
89 93
     def check

+ 25 - 0
db/migrate/20160607055850_change_events_order_to_events_list_order.rb

@@ -0,0 +1,25 @@
1
+class ChangeEventsOrderToEventsListOrder < ActiveRecord::Migration
2
+  def up
3
+    Agents::DataOutputAgent.find_each do |agent|
4
+      if value = agent.options.delete('events_order')
5
+        agent.options['events_list_order'] = value
6
+        agent.save!(validate: false)
7
+      end
8
+    end
9
+  end
10
+
11
+  def down
12
+    Agents::DataOutputAgent.transaction do
13
+      Agents::DataOutputAgent.find_each do |agent|
14
+        if agent.options['events_order']
15
+          raise ActiveRecord::IrreversibleMigration, "Cannot revert migration because events_order is configured"
16
+        end
17
+
18
+        if value = agent.options.delete('events_list_order')
19
+          agent.options['events_order'] = value
20
+          agent.save!(validate: false)
21
+        end
22
+      end
23
+    end
24
+  end
25
+end

+ 11 - 5
spec/models/agents/data_output_agent_spec.rb

@@ -142,7 +142,7 @@ describe Agents::DataOutputAgent do
142 142
           "url" => "http://imgs.xkcd.com/comics/evolving0.png",
143 143
           "title" => "Evolving yet again with a past date",
144 144
           "date" => '2014/05/05',
145
-          "hovertext" => "Something else"
145
+          "hovertext" => "A small text"
146 146
         }
147 147
       end
148 148
 
@@ -166,7 +166,7 @@ describe Agents::DataOutputAgent do
166 166
 
167 167
            <item>
168 168
             <title>Evolving yet again with a past date</title>
169
-            <description>Secret hovertext: Something else</description>
169
+            <description>Secret hovertext: A small text</description>
170 170
             <link>http://imgs.xkcd.com/comics/evolving0.png</link>
171 171
             <pubDate>#{Time.zone.parse(event3.payload['date']).rfc2822}</pubDate>
172 172
             <guid isPermaLink="false">#{event3.id}</guid>
@@ -216,7 +216,7 @@ describe Agents::DataOutputAgent do
216 216
           'items' => [
217 217
             {
218 218
               'title' => 'Evolving yet again with a past date',
219
-              'description' => 'Secret hovertext: Something else',
219
+              'description' => 'Secret hovertext: A small text',
220 220
               'link' => 'http://imgs.xkcd.com/comics/evolving0.png',
221 221
               'guid' => {"contents" => event3.id, "isPermaLink" => "false"},
222 222
               'pubDate' => Time.zone.parse(event3.payload['date']).rfc2822,
@@ -244,14 +244,20 @@ describe Agents::DataOutputAgent do
244 244
 
245 245
       describe 'ordering' do
246 246
         before do
247
-          agent.options['events_order'] = ['{{title}}']
247
+          agent.options['events_order'] = ['{{hovertext}}']
248
+          agent.options['events_list_order'] = ['{{title}}']
248 249
         end
249 250
 
250 251
         it 'can reorder the events_to_show last events based on a Liquid expression' do
252
+          agent.options['events_to_show'] = 2
253
+          asc_content, _status, _content_type = agent.receive_web_request({ 'secret' => 'secret2' }, 'get', 'application/json')
254
+          expect(asc_content['items'].map {|i| i["title"] }).to eq(["Evolving", "Evolving again"])
255
+
256
+          agent.options['events_to_show'] = 40
251 257
           asc_content, _status, _content_type = agent.receive_web_request({ 'secret' => 'secret2' }, 'get', 'application/json')
252 258
           expect(asc_content['items'].map {|i| i["title"] }).to eq(["Evolving", "Evolving again", "Evolving yet again with a past date"])
253 259
 
254
-          agent.options['events_order'] = [['{{title}}', 'string', true]]
260
+          agent.options['events_list_order'] = [['{{title}}', 'string', true]]
255 261
 
256 262
           desc_content, _status, _content_type = agent.receive_web_request({ 'secret' => 'secret2' }, 'get', 'application/json')
257 263
           expect(desc_content['items']).to eq(asc_content['items'].reverse)